Skip to content

feat: enable streamable http server for osv#14

Merged
yrobla merged 5 commits intomainfrom
add_streamable_http
Jun 17, 2025
Merged

feat: enable streamable http server for osv#14
yrobla merged 5 commits intomainfrom
add_streamable_http

Conversation

@yrobla
Copy link
Copy Markdown
Contributor

@yrobla yrobla commented Jun 16, 2025

add an env var that allows to switch mcp server mode, from sse to streamable http mode

add an env var that allows to switch mcp server mode, from
sse to streamable http mode
@yrobla yrobla requested review from JAORMX and Copilot June 16, 2025 12:00

This comment was marked as outdated.

@yrobla yrobla requested review from dmjb and rdimitrov June 16, 2025 12:01
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@yrobla yrobla requested a review from Copilot June 16, 2025 12:01

This comment was marked as outdated.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@yrobla yrobla requested a review from Copilot June 16, 2025 12:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds an environment‐variable switch to start the MCP server in either SSE or streamable HTTP mode.

  • Introduce ServeHTTPStream in pkg/mcp/server.go with streamable‐HTTP transport and heartbeat
  • Enhance handleQueryVulnerabilitiesBatch to validate argument structure
  • Wire MCP_TRANSPORT_MODE in cmd/server/main.go to select between SSE and stream modes and bump related dependencies

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
pkg/mcp/server.go Added ServeHTTPStream, heartbeat config, and improved batch‐query args parsing
cmd/server/main.go Added MCP_TRANSPORT_MODE env var and switch logic for transport modes
go.mod Bumped mcp-go and spf13/cast versions
Comments suppressed due to low confidence (4)

pkg/mcp/server.go:167

  • [nitpick] Consider making the endpoint path configurable instead of hard-coding "/mcp", so it can be adjusted without code changes.
server.WithEndpointPath("/mcp"),

pkg/mcp/server.go:231

  • [nitpick] The error message "Invalid arguments format" is quite generic; consider adding context, e.g. expected a map[string]interface{}.
if !ok {

pkg/mcp/server.go:164

  • Update documentation (README or comments) to include the new "streamable HTTP" mode and describe how to configure it via MCP_TRANSPORT_MODE.
log.Printf("Starting OSV MCP server (Streamable HTTP) on %s", addr)

pkg/mcp/server.go:163

  • There are no tests covering ServeHTTPStream; consider adding unit or integration tests to verify its behavior and heartbeat functionality.
func (s *Server) ServeHTTPStream(addr string) error {

Comment on lines +63 to +66
case "stream":
errChan <- mcpServer.ServeHTTPStream(*addr)
default:
log.Printf("Starting OSV MCP server (SSE) on %s", *addr)
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Extract the mode string literals ("stream", "sse") into named constants to avoid typos and improve maintainability.

Suggested change
case "stream":
errChan <- mcpServer.ServeHTTPStream(*addr)
default:
log.Printf("Starting OSV MCP server (SSE) on %s", *addr)
case ModeStream:
errChan <- mcpServer.ServeHTTPStream(*addr)
default:
log.Printf("Starting OSV MCP server (%s) on %s", ModeSSE, *addr)

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@yrobla yrobla force-pushed the add_streamable_http branch from 9c26711 to 8cefde4 Compare June 17, 2025 06:59
@yrobla yrobla requested a review from eleftherias June 17, 2025 08:43
Copy link
Copy Markdown
Contributor

@eleftherias eleftherias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one non-blocking comment. Also should we document this option in the README or is it not ready for general usage yet?

case "stream":
errChan <- mcpServer.ServeHTTPStream(*addr)
default:
log.Printf("Starting OSV MCP server (SSE) on %s", *addr)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This message gets printed twice because it's also printed in ServeSSE

@yrobla yrobla merged commit 005ab62 into main Jun 17, 2025
4 checks passed
@dmjb dmjb deleted the add_streamable_http branch June 17, 2025 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants